Skip to content

fix(supervisor-network): omit absent L7 selectors from policy data - #3200

Merged
johntmyers merged 1 commit into
NVIDIA:mainfrom
shiju-nv:fix/3092-omit-absent-l7-selectors
Sep 5, 2026
Merged

fix(supervisor-network): omit absent L7 selectors from policy data#3200
johntmyers merged 1 commit into
NVIDIA:mainfrom
shiju-nv:fix/3092-omit-absent-l7-selectors

Conversation

@shiju-nv

@shiju-nv shiju-nv commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Omit absent protobuf L7 scalar selectors when constructing OPA policy data. This gives protobuf and YAML inputs the same selector-key presence semantics, preventing an otherwise valid protocol rule from being classified as a mixed selector family because proto3 represented unrelated fields as empty strings.

Related Issue

Part of #3092.

Changes

  • Build the protobuf-derived allow object from selectors that are actually present instead of inserting empty method, path, command, operation_type, and operation_name values.
  • Preserve non-empty scalar selectors, fields, query, and params without changing the policy schema or enforcement rules.
  • Add direct projection tests for a GraphQL-over-WebSocket rule and a REST rule.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@shiju-nv
fix(supervisor-network): omit absent L7 selectors from policy data
9cc82c6 
Build protobuf-derived allow objects from selectors that are actually present. This keeps protobuf and YAML policy validation aligned for protocols such as GraphQL over WebSocket.

Signed-off-by: Shiju <shiju@nvidia.com>
@johntmyers johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage labels Sep 5, 2026
johntmyers
johntmyers reviewed Sep 5, 2026
View reviewed changes

@johntmyers johntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gator-agent

PR Review Status

This focused fix is project-valid under #3092. The initial code review found no blocking defects: the protobuf projection now omits absent scalar selector keys while preserving populated scalar and structured selectors, matching the key-presence semantics consumed by L7 validation.

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None

Non-blocking suggestions:

  • None
Gator metadata
  • Validation: Focused one-file policy correctness fix linked to #3092
  • Docs: Not needed because this changes internal protobuf-to-OPA projection semantics without changing the user-facing policy contract
  • Checks: Branch Checks and Helm Lint are green for the current head; required E2E dispatch is pending
  • E2E: test:e2e required for policy enforcement behavior and applied for current-head dispatch
  • Head SHA: 9cc82c63041aa8ee8327229f7e1299e42edbebf8
  • Base SHA: 592df3e01489b3fd2af5573e1feb043108596aed
  • Merge base SHA: 592df3e01489b3fd2af5573e1feb043108596aed
  • Patch ID: c56d9f9b78da4a7597a0a137dd824f0ae43529aa
  • Gator payload: 8
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Label test:e2e applied for 9cc82c6. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status labels Sep 5, 2026
@johntmyers
johntmyers added this pull request to the merge queue Sep 5, 2026
@johntmyers johntmyers added gator:merge-ready and removed gator:approval-needed Gator completed review; maintainer approval needed labels Sep 5, 2026
Merged via the queue into NVIDIA:main with commit b9c7d5c Sep 5, 2026
115 of 119 checks passed
@johntmyers

johntmyers commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: Gator review found no blocking defects, required E2E completed, and maintainer approval was present before merge.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

Gator metadata
  • Head SHA: 9cc82c63041aa8ee8327229f7e1299e42edbebf8
  • Gator payload: 8
  • Previous state: gator:merge-ready
  • Final state: merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johntmyers johntmyers johntmyers approved these changes
@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@sjenning sjenning Awaiting requested review from sjenning sjenning is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

Assignees

No one assigned

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shiju-nv @johntmyers